fix(helm): restore backward-compat discord rendering for per-agent configs#417
Conversation
Pre-Review Self-CheckIssue 驗收（對照 #416）
Code 品質
整合檢查
安全性
|
…nfigs When upgrading from 0.7.6 to 0.7.7, per-agent discord configs that lack the new discord.enabled field fail to render the [discord] TOML section, causing pods to crash with 'no adapter configured'. Replace the strict boolean check with a backward-compatible condition that also considers the presence of botToken, matching the intent of openabdev#394. Closes openabdev#416
JARVIS-coding-Agent
force-pushed
the
fix/helm-discord-backward-compat
branch
from
d18b00a to
081e46d
Compare
…OTES Replace the looser `or (enabled, botToken)` check with the same `botToken`-gated condition used by the other three templates, preventing configmap from rendering [discord] when no token is available. Also switches pipe syntax to function-call syntax for consistency. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
Reviewer 的兩點都準確，已在 69c2aa5 修正： 🔴 Suggested changes — 條件對齊 條件改回與 原本的 🟡 NIT — pipe 語法 同時換成 function call 語法 Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com |
Update test fixtures to supply discord.botToken where [discord] section is expected to render, matching the condition aligned with secret.yaml and deployment.yaml. Replace the "enabled=true without botToken renders" case with its inverse to document the new gate semantics. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
pahud
left a comment
pahud
left a comment
There was a problem hiding this comment.
Thanks for identifying the backward-compat issue — the root cause analysis is spot on.
However, we'd prefer to solve this with explicit defaults rather than inference logic. The botToken-presence fallback adds implicit magic that's hard to reason about and maintain.
Preferred approach: Set discord.enabled: true and slack.enabled: true in the chart's values.yaml defaults. This way:
- Template keeps the strict
enabledcheck — one code path, no guessing - Old 0.7.6 values files (no
enabledfield) inherit the defaulttruefrom the chart → backward-compat for free - Explicit opt-out via
enabled: falsewhen needed - Missing
botTokenwithenabled: true→ loud startup error instead of silent skip (better DX)
Closing this in favor of that approach.
|
Closing in favor of the explicit-defaults approach described in the review. Will open a new PR with |
2 participants
Context
Upgrading from chart 0.7.6 → 0.7.7 causes per-agent (non-default) deployments to crash with
no adapter configuredbecause the[discord]TOML section is not rendered for agents that lack the newdiscord.enabledfield. This contradicts the backward-compatible intent of #394.Closes #416
Summary
Replace the strict
($cfg.discord).enabledboolean check intemplates/configmap.yamlwith a backward-compatible condition that also considers the presence ofbotToken, so existing 0.7.6 values files work without modification on upgrade.Changes
charts/openab/templates/configmap.yaml: one-line change to the discord rendering conditiondiscord.enableddiscord.botTokentruefalseDiscord Discussion URL: https://discord.com/channels/1491295327620169908/1494495364655612095